Adding an optional version env var for the version endpoint to return#121
Adding an optional version env var for the version endpoint to return#121alexcottner wants to merge 1 commit intomainfrom
Conversation
5671f51 to
e318072
Compare
| :rtype: dict or None | ||
| """ | ||
| version_json = os.path.join(root, "version.json") | ||
| version_env_var = os.getenv("DOCKERFLOW_VERSION") |
There was a problem hiding this comment.
Why add this additional source of information instead of using the version.json file?
There was a problem hiding this comment.
We adjusted the remote-settings release process to re-tag the existing image instead of building a fresh one (link). This reduces our release times and allows us to use the exact same image that's been working great already in dev/stage.
But, the version file doesn't then get rebuilt so our version endpoint is incorrect. My best ideas to solve this were:
- Allow for an optional version parameter to be sent (this PR basically)
- Update and mount a configmap for the
version.jsonfile on deploy
This felt a little easier to me but I'm open to option 2.
There was a problem hiding this comment.
I think sliding the version in outside of the version.json file doesn't make sense and breaks the contract of version.json.
I think it'd be better to have a short Dockerfile template, expand the template with the image details of the stage image you're using, and then COPY the updated version.json file.
You should talk with @grahamalama and see if that's something we can put in deploy-actions.
There was a problem hiding this comment.
🤔 Yeah that's another option and is pretty simple to implement
tests/core/test_version.py
Outdated
| def test_env_var_override(tmpdir): | ||
| content = {"spam": "eggs"} | ||
| mock.patch.dict(os.environ, { "DOCKERFLOW_VERSION": "foo"}) | ||
| version_json = tmpdir.join("version.json") |
There was a problem hiding this comment.
This library uses pytest, so you should use its monkeypatching:
https://docs.pytest.org/en/stable/how-to/monkeypatch.html
| def test_env_var_override(tmpdir): | |
| content = {"spam": "eggs"} | |
| mock.patch.dict(os.environ, { "DOCKERFLOW_VERSION": "foo"}) | |
| version_json = tmpdir.join("version.json") | |
| def test_env_var_override(tmpdir, monkeypatch): | |
| content = {"spam": "eggs"} | |
| monkeypatch.setenv("DOCKERFLOW_VERSION", "foo") | |
| version_json = tmpdir.join("version.json") |
e318072 to
46ee69b
Compare
No description provided.